Make stream and memory resource explicit in hyperloglog APIs by PointKernel · Pull Request #9720 · NVIDIA/cccl

PointKernel · 2026-07-06T22:02:22Z

Description

This PR removes the default CUDA stream and default memory resource from cuda::experimental::cuco::hyperloglog and hyperloglog_ref, following the cuda::buffer design adopted for fixed_capacity_map in #9719:

All hyperloglog constructors now take cuda::stream_ref as the first argument and the memory resource as the second, matching cuda::buffer's argument order. The constructors that silently picked the default memory pool of device 0 are removed.
All host bulk APIs (clear, clear_async, add, add_async, merge, merge_async, estimate) on both hyperloglog and hyperloglog_ref take the stream as the first parameter, with no default.

It also applies the conventions established during the #7705 review to the hyperloglog headers:

_CCCL_HOST_API/_CCCL_HOST_DEVICE_API annotations on all hyperloglog member functions, which were previously unannotated constexpr.
_CCCL_TEMPLATE/_CCCL_REQUIRES instead of raw enable_if SFINAE for the constrained clear(_CG) and merge(_CG, ...) device overloads.
#if !_CCCL_COMPILER(NVRTC) guard around the host-only hyperloglog class and #if _CCCL_CUDA_COMPILATION() around the device kernels.
Granular per-symbol includes instead of the bulk <cuda/experimental/container.cuh>, <cuda/experimental/memory_resource.cuh>, and <cuda/experimental/stream.cuh> headers.
West const and no this-> prefixes for member access.

Checklist

New or existing tests cover these changes.
The documentation is up to date with these changes.

copy-pr-bot · 2026-07-06T22:02:25Z

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

PointKernel · 2026-07-06T22:03:28Z

/ok to test 84e48f4

coderabbitai · 2026-07-07T01:42:14Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 70a3b84d-8347-475c-adc9-a33b09ab079f

📥 Commits

Reviewing files that changed from the base of the PR and between 669032e and 333eb34.

📒 Files selected for processing (1)

cudax/include/cuda/experimental/__cuco/detail/hyperloglog/hyperloglog_impl.cuh

🚧 Files skipped from review as they are similar to previous changes (1)

cudax/include/cuda/experimental/__cuco/detail/hyperloglog/hyperloglog_impl.cuh

📝 Walkthrough

Summary by CodeRabbit

Breaking Changes
- HyperLogLog (hyperloglog and hyperloglog_ref) now requires explicit CUDA stream selection for construction and all stream-enqueued operations. Pass ::cuda::stream_ref as the first argument (default-stream overloads removed and parameter order updated for clear, add, merge, and estimate).
Tests
- Updated HyperLogLog tests to use the stream- and memory-resource-aware estimator APIs across all affected cases, including pinned and device-side paths.

Walkthrough

HyperLogLog host and reference APIs now require explicit ::cuda::stream_ref arguments, with stream-first ordering across construction, clear/add/merge/estimate calls. The implementation and kernel headers also received small cleanup and guard changes, and the tests were updated to use stream-aware construction and memory pools.

Changes

HyperLogLog Stream API Rework

Layer / File(s)	Summary
hyperloglog_ref stream-first API and constraints `cudax/include/cuda/experimental/__cuco/hyperloglog_ref.cuh`	`clear`, `add`, `merge`, and `estimate` now take `::cuda::stream_ref` first with no default stream arguments; cooperative-group overloads use `CCCL_REQUIRES`; related includes were updated.
hyperloglog host wrapper stream-first API `cudax/include/cuda/experimental/__cuco/hyperloglog.cuh`	Constructors and host methods now require explicit stream arguments first, accessors and sketch helpers gained host/device annotations, precision helpers were requalified, and internal include dependencies were switched.
Impl and kernel cleanup, guard adjustments `cudax/include/cuda/experimental/__cuco/detail/hyperloglog/default_policy.cuh`, `cudax/include/cuda/experimental/__cuco/detail/hyperloglog/finalizer.cuh`, `cudax/include/cuda/experimental/__cuco/detail/hyperloglog/hyperloglog_impl.cuh`, `cudax/include/cuda/experimental/__cuco/detail/hyperloglog/kernels.cuh`	Redundant `this->` qualifiers are removed, one include is dropped, a local declaration style changes, and kernel preprocessor guards plus a toolkit-conditional remainder branch are restructured.
Tests updated for stream-aware estimator `cudax/test/cuco/hyperloglog/test_hyperloglog.cu`	Tests now construct estimators with explicit streams and memory pools, pass streams into add/clear/estimate calls, and use a shared `scaled_index` functor in the Spark parity case.

Suggested reviewers: caugonnet, srinivasyadav18, davebayer, Jacobfaib, sleeepyjack

_{Comment @coderabbitai help to get the list of available commands.}

coderabbitai

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

cudax/include/cuda/experimental/__cuco/hyperloglog.cuh (1)

154-169: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

important: constructor still returns before the initial clear completes The three constructors enqueue clear_async(__stream) and return without synchronizing, so the @note claiming stream sync is stale. Either call clear(__stream) here or update the note to say construction is asynchronous and the caller must synchronize before using the object.

🧹 Nitpick comments (3)

cudax/test/cuco/hyperloglog/test_hyperloglog.cu (1)

79-88: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

suggestion: the ::cuda::stream{device_ref{0}} + device_default_memory_pool(device_ref{0}) setup is repeated identically across all five tests. Could extract into a shared fixture/helper to reduce duplication, but not blocking.

Also applies to: 122-146, 189-199, 224-230, 251-258
cudax/include/cuda/experimental/__cuco/hyperloglog_ref.cuh (1)
231-231: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

suggestion: Stray // Review: note sits inside the //! doxygen block and will bleed into the generated docs / is a leftover artifact. Drop it (or convert to a tracked TODO).
-  //! `@throw` If sketch_bytes() != __other.sketch_bytes()
-  //!
-  // Review: For host-side merges, consider validating `__other` is on the same device.
-  //! `@tparam` _OtherScope Thread scope of `other` estimator
+  //! `@throw` If sketch_bytes() != __other.sketch_bytes()
+  //!
+  //! `@tparam` _OtherScope Thread scope of `other` estimator
cudax/include/cuda/experimental/__cuco/hyperloglog.cuh (1)

159-166: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

suggestion: add #include <cuda/__utility/no_init.h> here; ::cuda::no_init is used directly and shouldn’t depend on device_memory_pool.h for availability.

Source: Coding guidelines

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 87f29846-e10b-4203-be12-9e09a637769c

📥 Commits

Reviewing files that changed from the base of the PR and between 48ff8d1 and 84e48f4.

📒 Files selected for processing (7)

cudax/include/cuda/experimental/__cuco/detail/hyperloglog/default_policy.cuh
cudax/include/cuda/experimental/__cuco/detail/hyperloglog/finalizer.cuh
cudax/include/cuda/experimental/__cuco/detail/hyperloglog/hyperloglog_impl.cuh
cudax/include/cuda/experimental/__cuco/detail/hyperloglog/kernels.cuh
cudax/include/cuda/experimental/__cuco/hyperloglog.cuh
cudax/include/cuda/experimental/__cuco/hyperloglog_ref.cuh
cudax/test/cuco/hyperloglog/test_hyperloglog.cu

…nclude, buffer-based tests

github-actions · 2026-07-08T21:38:54Z

🥳 CI Workflow Results

🟩 Finished in 1h 07m: Pass: 100%/55 | Total: 5h 27m | Max: 1h 07m | Hits: 87%/40789

See results here.

PointKernel added 2 commits July 6, 2026 21:48

Make stream and memory resource explicit in hyperloglog APIs

ae3b6ea

Apply cudax cuco conventions to hyperloglog headers

84e48f4

github-project-automation Bot added this to CCCL Jul 6, 2026

github-project-automation Bot moved this to Todo in CCCL Jul 6, 2026

cccl-authenticator-app Bot moved this from Todo to In Progress in CCCL Jul 6, 2026

PointKernel added cuco cuCollections cudax Feature intended for the cudax experimental library labels Jul 6, 2026

This comment has been minimized.

Sign in to view

PointKernel marked this pull request as ready for review July 7, 2026 01:36

PointKernel requested a review from a team as a code owner July 7, 2026 01:36

PointKernel requested review from caugonnet and davebayer July 7, 2026 01:36

cccl-authenticator-app Bot moved this from In Progress to In Review in CCCL Jul 7, 2026

PointKernel requested review from shwina, sleeepyjack and srinivasyadav18 and removed request for shwina July 7, 2026 01:36

coderabbitai Bot reviewed Jul 7, 2026

View reviewed changes

PointKernel added 2 commits July 7, 2026 18:25

Address review feedback: stream-ordered ctor docs, granular no_init i…

653cbea

…nclude, buffer-based tests

Merge remote-tracking branch 'upstream/main' into hll-explicit-stream-mr

e981a78

This comment has been minimized.

Sign in to view

Jacobfaib approved these changes Jul 7, 2026

View reviewed changes

Comment thread cudax/include/cuda/experimental/__cuco/detail/hyperloglog/kernels.cuh Outdated

Comment thread cudax/include/cuda/experimental/__cuco/hyperloglog.cuh Outdated

PointKernel added 2 commits July 8, 2026 17:07

Add #else/#endif comments and hide defaulted special members from ABI

03f8474

Merge remote-tracking branch 'upstream/main' into hll-explicit-stream-mr

669032e

This comment has been minimized.

Sign in to view

PointKernel added 2 commits July 8, 2026 19:25

Use to_address instead of subscripting the iterator in host code

412bbd9

Merge remote-tracking branch 'upstream/main' into hll-explicit-stream-mr

333eb34

srinivasyadav18 approved these changes Jul 8, 2026

View reviewed changes

sleeepyjack approved these changes Jul 8, 2026

View reviewed changes

PointKernel merged commit e1185c2 into NVIDIA:main Jul 9, 2026
77 of 78 checks passed

PointKernel deleted the hll-explicit-stream-mr branch July 9, 2026 16:05

github-project-automation Bot moved this from In Review to Done in CCCL Jul 9, 2026

sleeepyjack mentioned this pull request Jul 9, 2026

Adapt HLL backend to CCCL stream-first API apache/datasketches-cuda#3

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Make stream and memory resource explicit in hyperloglog APIs#9720

Make stream and memory resource explicit in hyperloglog APIs#9720
PointKernel merged 8 commits into
NVIDIA:mainfrom
PointKernel:hll-explicit-stream-mr

PointKernel commented Jul 6, 2026

Uh oh!

copy-pr-bot Bot commented Jul 6, 2026

Uh oh!

PointKernel commented Jul 6, 2026

Uh oh!

This comment has been minimized.

coderabbitai Bot commented Jul 7, 2026 •

edited

Loading

Summary by CodeRabbit

Walkthrough

Changes

Uh oh!

coderabbitai Bot left a comment

Uh oh!

This comment has been minimized.

Uh oh!

Uh oh!

This comment has been minimized.

github-actions Bot commented Jul 8, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

PointKernel commented Jul 6, 2026

Description

Checklist

Uh oh!

copy-pr-bot Bot commented Jul 6, 2026

Uh oh!

PointKernel commented Jul 6, 2026

Uh oh!

This comment has been minimized.

coderabbitai Bot commented Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Walkthrough

Changes

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

Uh oh!

Uh oh!

This comment has been minimized.

github-actions Bot commented Jul 8, 2026

🥳 CI Workflow Results

🟩 Finished in 1h 07m: Pass: 100%/55 | Total: 5h 27m | Max: 1h 07m | Hits: 87%/40789

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

coderabbitai Bot commented Jul 7, 2026 •

edited

Loading